[BUG](exec) fix coalesce function output null#63092
Open
qzsee wants to merge 3 commits into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
HappenLee
changed the title
Contributor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
The implementation change is small and directly addresses the reported COALESCE value-pollution path, but this PR currently has no regression or unit test for the bug it fixes. Because this is BE expression correctness code, Doris review standards require a targeted test before merge.
Critical checkpoint conclusions:
- Goal/test: The code appears to accomplish the stated goal for scalar non-string COALESCE results by avoiding arithmetic with null-position payloads, but no test proves the reported nullable DOUBLE/NaN case or prevents regression.
- Scope: The code change is focused and minimal.
- Concurrency/lifecycle: Not applicable; this is per-block expression evaluation with no new shared state or lifecycle management.
- Configuration/compatibility/protocol: Not applicable; no config, persisted format, or FE-BE protocol changes.
- Parallel paths: String/array/map/struct/JSON and bitmap paths already use guarded insertion/assignment; the changed scalar path is the relevant divergent path.
- Conditions/error handling: No new error path; the new guard is the necessary data-correctness condition.
- Tests/results: Missing targeted regression/unit coverage and no expected result update was included.
- Observability/transactions/data writes: Not applicable.
- Performance: The new branch is in a hot scalar loop but is justified by correctness; no obvious blocking performance issue found.
Focus points: No additional user-provided review focus was supplied.
| column_raw_data[row] * | ||
| typename ColumnType::value_type(!(null_map_data[row] | filled_flag[row])); | ||
| if (!(null_map_data[row] | filled_flag[row])) { | ||
| result_raw_data[row] = column_raw_data[row]; |
Contributor
There was a problem hiding this comment.
This bug fix needs a targeted regression test. The PR description describes a concrete data-correctness failure where COALESCE(nullable_double, 0) can return NULL after a null-position payload contains NaN, but the PR only changes BE code and does not add a case under the existing conditional function/coalesce tests (or a BE unit test) that fails before this change and passes after it. Please add coverage for the nullable floating-point path so this does not regress.
yiguolei
reviewed
May 9, 2026
| result_raw_data[row] += | ||
| column_raw_data[row] * | ||
| typename ColumnType::value_type(!(null_map_data[row] | filled_flag[row])); | ||
| if (!(null_map_data[row] | filled_flag[row])) { |
Contributor
There was a problem hiding this comment.
add test please and also check the document
Contributor
|
run buildall |
Contributor
TPC-H: Total hot run time: 29271 ms |
Contributor
TPC-DS: Total hot run time: 170437 ms |
|
run buildall |
Contributor
TPC-H: Total hot run time: 29699 ms |
Contributor
TPC-DS: Total hot run time: 170254 ms |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
skip buildall |
Contributor
|
PR approved by anyone and no changes requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Example: COALESCE(same_department_income_amount, 0) ==> outputs NULL (where same_department_income_amount is of type double).
When assigning the value to the result column in the computation, the assignment is done unconditionally (forced), as in:
result_raw_data[row] += column_raw_data[row] * typename ColumnType::value_type(!(null_map_data[row] | filled_flag[row]));If the argument column column_raw_data's null_map[row] is 1, then the value stored in column_raw_data[row] is garbage data. This garbage may contain values such as NaN. If a preceding argument of COALESCE happens to be assigned NaN, then during subsequent assignments we run into cases like:
0 * NaN = NaN
num + NaN = NaN
so the assigned result also becomes NaN, which causes value pollution.
By rights the final output should also be NaN, but what is actually returned is NULL. The reason is that during result serialization/output, NaN values are emitted as NULL.
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)